Skip to content

Fix imageData.data type#189

Merged
nojaf merged 4 commits intorescript-lang:mainfrom
alex35mil:image-data-type
Jan 9, 2026
Merged

Fix imageData.data type#189
nojaf merged 4 commits intorescript-lang:mainfrom
alex35mil:image-data-type

Conversation

@alex35mil
Copy link
Contributor

https://developer.mozilla.org/en-US/docs/Web/API/ImageData/data

I changed it to Uint8ClampedArray.t because initially it was array<int>.

Copy link
Member

@nojaf nojaf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be great if you could add a unit test here as well.

@@ -11,7 +11,7 @@ external make: (~sw: int, ~sh: int, ~settings: imageDataSettings=?) => imageData
*/
@new
external make2: (
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please rename the function as well while you are here:

Suggested change
external make2: (
external makeWithData: (

@alex35mil
Copy link
Contributor Author

@nojaf both done

Copy link
Member

@nojaf nojaf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wonderful! Thanks very much.
There should be a new version once CI is done on main.

@@ -0,0 +1,5 @@
open DOMAPI
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you rename file to ImageData_test.res please

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, done

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thx!

@nojaf nojaf merged commit 469231f into rescript-lang:main Jan 9, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants